Expose a platform-specific line separator #449
Conversation
|
Hmm @fzhinkin will this get merged soon? |
fzhinkin
force-pushed
the
448-system-line-separator
branch
from
d878fa0 to
4a8e612
Compare
| try { | ||
| os.EOL | ||
| } catch (_: Throwable) { | ||
| "\r\n" |
There was a problem hiding this comment.
Is it somehow justified that we use \r\n as a fallback?
There was a problem hiding this comment.
HTML uses CRLF as a line separator, so I thought it's a good idea to fall back to it. But it has to be explicitly specified.
There was a problem hiding this comment.
Oh, wait, it's already documented and says that the separator is different :D
There was a problem hiding this comment.
@JSMonk does such a choice make any sense? Or it's better to use \n?
There was a problem hiding this comment.
@fzhinkin Does it make sense to check whether navigator.platform is Windows, and if so, use \r\n; otherwise use \n for Android, Linux, macOS, and pretty much every other Os?
There was a problem hiding this comment.
I'm looking at this from the following perspective: a browser and JS code executed inside is aimed for interaction with Internet, and the line separator used for documents shared and transferred there is somewhat unrelated to the operating system where the browser is running. That's why I picked the line separator from HTTP standard.
But IDK how far fetched it is from the actual use cases.
There was a problem hiding this comment.
Browsers have access to the file system now, and likely should represent the OS on which they're running.
There was a problem hiding this comment.
Fair enough. I added window.navigator.platform-specific line separator for both Js and WasmJs
| package kotlinx.io | ||
|
|
||
| /** | ||
| * Sequence of characters used as a line separator by the underlying platform. |
There was a problem hiding this comment.
Maybe it's better to explicitly write about it in a specific platform, like:
Sequence of characters used as a line separator in Apple targets?
There was a problem hiding this comment.
I would prefer having the same brief kdoc line across all actualizations, and only add platform specific details in the expanded version of a doc. But that's a personal preference, I don't have strong arguments to support it (docs will be different anyway). So we can try your suggestion.
There was a problem hiding this comment.
Depending on the target, I tend to end the phrase "Sequence of characters used as a line separator ... " with:
- "... in JS environments"
- "... by Windows"
- "... on Apple targets"
- "... by Unix platforms"
- "... by the JVM"
WDYT? The lack of consistency bugs me, and writing "on XXX targets" everywhere feels off.
Remove dependency on NodeJs modules from SystemLineSeparator. Also, run wasmJs tests in browser, and migrate isWindows property to NodeJs-agnostic API.
| (typeof navigator !== "undefined" && navigator.platform) | ||
| || (typeof window !== "undefined" && typeof window.navigator !== "undefined" && window.navigator.platform) | ||
| || "unknown" | ||
| """ |
There was a problem hiding this comment.
If you heard a noise somewhere outside, it was most likely me screaming. :|
There was a problem hiding this comment.
Checking both navigator and window.navigator is usually unnecessary.
In browsers, navigator is a global object and window.navigator
references the exact same object. Accessing either one yields the same value,
just like other globals attached to window.
Example to demonstrate this more clearly:
var globalVar = "I'm global!";
console.log(globalVar); // Works
console.log(window.globalVar); // Also works and refers to the exact same variableI would suggest only checking navigator.platform.
But it doesn't really matter ;) so yeah.
Closes #448